Conversation
- Updated base Node.js version from 22 to 24 in Dockerfile.node.base.dockerfile and related build scripts. - Changed Node.js version in pnpm-workspace.yaml files to 24.14.0 for compatibility. - Updated Rust toolchain version from 1.92 to 1.93 in rust-toolchain.toml and Cargo.toml files
github-actions
bot
added
dependencies
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
system-tests/src/tests/exams/exam-instructions.spec.ts (2)
40-44: Normalize the empty-block selector; avoid escaped duplicate variant.The escaped aria-label selector at Lines 41-43 is hard to read and maintain. Reuse one canonical selector string for both interactions.
Suggested change
+ const emptyBlockSelector = + `[aria-label="Empty block; start writing or type forward slash to choose a block"]` + await page - .locator(`[aria-label="Empty block; start writing or type forward slash to choose a block"]`) + .locator(emptyBlockSelector) .fill("These are the instructions") await page.getByText("These are the instructions").press("Enter") await page - .locator( - `[aria-label="Empty\\ block\\;\\ start\\ writing\\ or\\ type\\ forward\\ slash\\ to\\ choose\\ a\\ block"]`, - ) + .locator(emptyBlockSelector) .pressSequentially("/")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@system-tests/src/tests/exams/exam-instructions.spec.ts` around lines 40 - 44, The aria-label selector used in the locator call is duplicated with hard-to-read escaping; extract the canonical selector string into a single constant (e.g., const EMPTY_BLOCK_SELECTOR = `[aria-label="Empty block; start writing or type forward slash to choose a block"]`) and replace the inline duplicated escaped selector in the locator(...) call and any other uses (like the pressSequentially("/") interaction) with that constant (update the locator invocation that calls .locator(EMPTY_BLOCK_SELECTOR) and then .pressSequentially("/")). Ensure the constant uses the unescaped, human-readable text so both locator and subsequent interactions reuse the same selector.
39-39: Use keyboard Enter instead of text-locator Enter to avoid focus ambiguity.At Line 39,
getByText("These are the instructions").press("Enter")can become flaky if multiple matches appear or focus shifts in the editor. Prefer pressing Enter on the active editor focus.Suggested change
- await page.getByText("These are the instructions").press("Enter") + await page.keyboard.press("Enter")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@system-tests/src/tests/exams/exam-instructions.spec.ts` at line 39, The current call page.getByText("These are the instructions").press("Enter") is flaky due to focus ambiguity; instead ensure the editor has focus and use keyboard input: focus the editor locator (e.g., page.getByRole("textbox") or the specific editor locator) via its .focus() method, then invoke page.keyboard.press("Enter") to send the Enter keystroke; update the test to replace the getByText(...).press("Enter") usage with the two-step focus + keyboard.press flow to avoid ambiguous text-locator pressing.services/cms/package.json (1)
30-30: Make@next/bundle-analyzerversioning consistent withnext.Line 30 pins
@next/bundle-analyzerto16.1.6while line 65 allowsnextto float with^16.1.6. These should use the same versioning strategy to avoid unintended version drift.♻️ Proposed fix
- "@next/bundle-analyzer": "16.1.6", + "@next/bundle-analyzer": "^16.1.6",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/cms/package.json` at line 30, The `@next/bundle-analyzer` dependency is pinned to "16.1.6" while the "next" dependency uses a caret range; make their versioning strategy consistent by updating the "@next/bundle-analyzer" entry in package.json to use the same semver style as "next" (e.g., change "@next/bundle-analyzer": "16.1.6" to "@next/bundle-analyzer": "^16.1.6"), or alternatively pin both to exact versions—ensure you update the "@next/bundle-analyzer" line to match whichever approach is used for "next".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@services/cms/package.json`:
- Line 30: The `@next/bundle-analyzer` dependency is pinned to "16.1.6" while the
"next" dependency uses a caret range; make their versioning strategy consistent
by updating the "@next/bundle-analyzer" entry in package.json to use the same
semver style as "next" (e.g., change "@next/bundle-analyzer": "16.1.6" to
"@next/bundle-analyzer": "^16.1.6"), or alternatively pin both to exact
versions—ensure you update the "@next/bundle-analyzer" line to match whichever
approach is used for "next".
In `@system-tests/src/tests/exams/exam-instructions.spec.ts`:
- Around line 40-44: The aria-label selector used in the locator call is
duplicated with hard-to-read escaping; extract the canonical selector string
into a single constant (e.g., const EMPTY_BLOCK_SELECTOR = `[aria-label="Empty
block; start writing or type forward slash to choose a block"]`) and replace the
inline duplicated escaped selector in the locator(...) call and any other uses
(like the pressSequentially("/") interaction) with that constant (update the
locator invocation that calls .locator(EMPTY_BLOCK_SELECTOR) and then
.pressSequentially("/")). Ensure the constant uses the unescaped, human-readable
text so both locator and subsequent interactions reuse the same selector.
- Line 39: The current call page.getByText("These are the
instructions").press("Enter") is flaky due to focus ambiguity; instead ensure
the editor has focus and use keyboard input: focus the editor locator (e.g.,
page.getByRole("textbox") or the specific editor locator) via its .focus()
method, then invoke page.keyboard.press("Enter") to send the Enter keystroke;
update the test to replace the getByText(...).press("Enter") usage with the
two-step focus + keyboard.press flow to avoid ambiguous text-locator pressing.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
services/cms/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlservices/headless-lms/Cargo.lockis excluded by!**/*.locksystem-tests/src/__screenshots__/quizzes/widget/multiple-choice-clickable.spec.ts/widget-multiple-choice-clickable-answered-desktop-regular.pngis excluded by!**/*.pngsystem-tests/src/__screenshots__/quizzes/widget/multiple-choice-row-widget.spec.ts/widget-multiple-choice-row-initial-desktop-regular.pngis excluded by!**/*.pngsystem-tests/src/__screenshots__/sidebar-toggle.spec.ts/block-list-desktop-regular.pngis excluded by!**/*.pngsystem-tests/src/__screenshots__/sidebar-toggle.spec.ts/block-menu-desktop-regular.pngis excluded by!**/*.pngsystem-tests/src/__screenshots__/sidebar-toggle.spec.ts/block-properties-desktop-regular.pngis excluded by!**/*.png
📒 Files selected for processing (11)
services/cms/package.jsonservices/cms/src/components/editors/GutenbergEditor.tsxservices/headless-lms/Cargo.tomlservices/headless-lms/server/Cargo.tomlservices/headless-lms/server/src/programs/start_server.rssystem-tests/src/tests/cms/converting-blocks.spec.tssystem-tests/src/tests/course-export.spec.tssystem-tests/src/tests/exams/exam-instructions.spec.tssystem-tests/src/tests/exams/grade-exams-manually.spec.tssystem-tests/src/tests/partners-block.spec.tssystem-tests/src/tests/research-form.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- services/headless-lms/server/Cargo.toml
# Conflicts: # services/headless-lms/Cargo.lock
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
system-tests/src/tests/user-research-consent.spec.ts (1)
57-59:⚠️ Potential issue | 🟠 MajorAssert dialog absence explicitly after re-login.
At Line 58, checking the “Organizations” heading does not prove the consent dialog is gone; this can pass as a false positive. Add an explicit negative assertion for the dialog.
Suggested patch
await page.locator("id=login-button").click() await expect(page.getByRole("heading", { name: "Organizations" })).toBeVisible() + await expect(page.getByTestId("research-consent-dialog")).not.toBeVisible()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@system-tests/src/tests/user-research-consent.spec.ts` around lines 57 - 59, After re-login (after interacting with "id=login-button" and asserting the "Organizations" heading), add an explicit negative assertion that the user-research consent dialog is not present (instead of relying on the heading check). Locate the consent dialog using the same selector used elsewhere in the test (e.g., the dialog role or its unique text/label) and assert its absence with a negative expectation (for example using page.getByRole('dialog', { name: /consent/i }) or the project's consent dialog selector and .not.toBeVisible() or .toHaveCount(0)); update the test around the "id=login-button" / "Organizations" assertions to include this explicit negative check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@system-tests/src/tests/user-research-consent.spec.ts`:
- Around line 57-59: After re-login (after interacting with "id=login-button"
and asserting the "Organizations" heading), add an explicit negative assertion
that the user-research consent dialog is not present (instead of relying on the
heading check). Locate the consent dialog using the same selector used elsewhere
in the test (e.g., the dialog role or its unique text/label) and assert its
absence with a negative expectation (for example using page.getByRole('dialog',
{ name: /consent/i }) or the project's consent dialog selector and
.not.toBeVisible() or .toHaveCount(0)); update the test around the
"id=login-button" / "Organizations" assertions to include this explicit negative
check.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
services/headless-lms/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
services/headless-lms/chatbot/Cargo.tomlservices/headless-lms/models/src/courses.rsservices/headless-lms/server/src/controllers/course_material/chatbot.rssystem-tests/src/tests/exams/grade-exams-manually.spec.tssystem-tests/src/tests/user-research-consent.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- system-tests/src/tests/exams/grade-exams-manually.spec.ts
- services/headless-lms/chatbot/Cargo.toml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
shared-module/packages/common/package.json (1)
78-78:⚠️ Potential issue | 🟠 Major
@types/nodemajor is still ahead of the pinned Node runtime major.At Line 78,
@types/nodeis^25.3.3while workspace Node is pinned to 24.x in this PR context. This can type-check APIs that are unavailable at runtime.#!/bin/bash set -euo pipefail echo "== Node version pins from workspace files ==" fd 'pnpm-workspace.yaml$' | while read -r f; do echo "-- $f" rg -n 'nodeVersion:' "$f" || true done echo echo "== `@types/node` in shared-module/packages/common/package.json ==" rg -n '"@types/node"\s*:' shared-module/packages/common/package.jsonSuggested patch
- "@types/node": "^25.3.3", + "@types/node": "^24.14.0",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shared-module/packages/common/package.json` at line 78, The `@types/node` dependency in shared-module/packages/common/package.json is set to "^25.3.3" which is ahead of the workspace Node runtime (pinned to 24.x); update the "@types/node" version to a 24.x-compatible release (e.g., "^24.0.0") in that package.json, run your install/type-check (pnpm install && pnpm -w -F common run typecheck or equivalent) to verify no new type errors, and commit the change so the dev typings match the pinned Node runtime.
🧹 Nitpick comments (2)
services/headless-lms/models/src/user_chapter_locking_statuses.rs (1)
236-307: Document side effects inget_or_init_all_for_courseThis public function now does more than “get/init”: it inserts missing statuses and may unlock chapters. Please add a short doc comment describing those side effects and when they happen, so callers don’t treat it as a read-only fetch.
As per coding guidelines: "Require consistent, minimal function documentation... include a short, plain-language description of what the function is for... Comments should add meaning rather than paraphrasing the name or implementation."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/headless-lms/models/src/user_chapter_locking_statuses.rs` around lines 236 - 307, Add a short doc comment above the public function get_or_init_all_for_course describing its side effects: it will insert missing user_chapter_locking_statuses for the course when course.chapter_locking_enabled is true, and it may call crate::chapters::unlock_first_chapters_for_user to unlock chapters (triggered when course locking is enabled and all existing statuses are ChapterLockingStatus::NotUnlockedYet), so callers should not assume this is a read-only fetch; mention the conditions under which each side effect occurs.services/main-frontend/package.json (1)
60-63: Keepreactandreact-domlocked to the same exact version.At Lines 60-63,
reactis exact (19.2.4) whilereact-domis ranged (^19.2.4). This can drift to different patch/minor builds over time.Suggested patch
- "react-dom": "^19.2.4", + "react-dom": "19.2.4",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/main-frontend/package.json` around lines 60 - 63, The package.json currently pins "react" to an exact version and "react-dom" with a caret, which can allow drift; update the dependency declaration for "react-dom" (the react-dom entry in package.json) to use the exact same version string as "react" (change "^19.2.4" to "19.2.4") so both "react" and "react-dom" are locked to identical versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/cms/package.json`:
- Line 86: The package.json dependency "@types/node" is pinned to ^25.3.3 which
mismatches the workspace Node runtime (nodeVersion 24.14.0); update the
"@types/node" entry in services/cms package.json to use the matching major/minor
version (e.g., ^24.14.0) so the types align with the pinned runtime and avoid
type/runtime incompatibilities.
In `@services/headless-lms/models/src/user_chapter_locking_statuses.rs`:
- Around line 557-642: The test
get_or_init_all_for_course_unlocks_first_chapter_when_all_not_unlocked_yet is
misleading because no exercises are created so multiple chapters could
auto-unlock; fix by making the setup match the intent: create at least one
exercise tied to chapter1 (e.g., insert an exercise for chapter1 via the same
crate::exercises insert helper) so only chapter1 should be unlocked, or
alternatively strengthen the assertions after calling get_or_init_all_for_course
to assert chapter1 is Unlocked and chapter2 remains NotUnlockedYet (use
ChapterLockingStatus variants and the chapter1/chapter2 ids and functions
get_or_init_all_for_course and ensure_not_unlocked_yet_status to locate the
logic).
In `@services/main-frontend/package.json`:
- Line 88: The dev dependency "@types/node" is set to ^25.3.3 but the workspace
Node runtime is pinned to 24.14.0; update the entry for "@types/node" in
package.json to use a 24.x caret range (e.g., "^24.0.0" or "^24.x") so the type
definitions match the pinned Node major and prevent invalid API type-checking
against Node 25.
In `@services/quizzes/package.json`:
- Line 47: The package.json contains an unsupported "export": "next export" npm
script (and next.config.js currently uses output: "standalone"); either remove
the "export" script and any tooling invoking it if you don't need static
exports, or change next.config.js output to "export" and replace the script with
a build-based flow (use "next build" and then serve the generated static output)
so you stop calling the removed CLI; update the package.json scripts accordingly
and ensure next.config.js's output setting (the output property) and any
deployment steps align (refer to the "export" script in package.json and the
output property in next.config.js).
---
Duplicate comments:
In `@shared-module/packages/common/package.json`:
- Line 78: The `@types/node` dependency in
shared-module/packages/common/package.json is set to "^25.3.3" which is ahead of
the workspace Node runtime (pinned to 24.x); update the "@types/node" version to
a 24.x-compatible release (e.g., "^24.0.0") in that package.json, run your
install/type-check (pnpm install && pnpm -w -F common run typecheck or
equivalent) to verify no new type errors, and commit the change so the dev
typings match the pinned Node runtime.
---
Nitpick comments:
In `@services/headless-lms/models/src/user_chapter_locking_statuses.rs`:
- Around line 236-307: Add a short doc comment above the public function
get_or_init_all_for_course describing its side effects: it will insert missing
user_chapter_locking_statuses for the course when course.chapter_locking_enabled
is true, and it may call crate::chapters::unlock_first_chapters_for_user to
unlock chapters (triggered when course locking is enabled and all existing
statuses are ChapterLockingStatus::NotUnlockedYet), so callers should not assume
this is a read-only fetch; mention the conditions under which each side effect
occurs.
In `@services/main-frontend/package.json`:
- Around line 60-63: The package.json currently pins "react" to an exact version
and "react-dom" with a caret, which can allow drift; update the dependency
declaration for "react-dom" (the react-dom entry in package.json) to use the
exact same version string as "react" (change "^19.2.4" to "19.2.4") so both
"react" and "react-dom" are locked to identical versions.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
services/cms/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlservices/example-exercise/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlservices/main-frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlservices/quizzes/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlservices/tmc/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlshared-module/packages/common/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
services/cms/package.jsonservices/example-exercise/package.jsonservices/headless-lms/models/src/user_chapter_locking_statuses.rsservices/main-frontend/package.jsonservices/quizzes/package.jsonservices/tmc/package.jsonshared-module/packages/common/package.jsonshared-module/packages/common/src/hooks/useConcurrencyThrottle.tsshared-module/packages/common/tests/setup-jest.js
🚧 Files skipped from review as they are similar to previous changes (1)
- services/example-exercise/package.json
| "@types/node": "^24.6.2", | ||
| "@types/jsdom": "^28.0.0", | ||
| "@types/lodash": "^4.17.24", | ||
| "@types/node": "^25.3.3", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Workspace configuration files =="
fd -e yaml -e yml | grep -E '(workspace|pnpm)' | head -20
echo
echo "== Content of pnpm-workspace.yaml if exists =="
if [ -f "pnpm-workspace.yaml" ]; then
cat pnpm-workspace.yaml
fi
echo
echo "== services/cms/package.json - `@types/node` line =="
rg -n '"@types/node"' services/cms/package.json -A 1 -B 1
echo
echo "== services/cms/package.json line 86 context =="
sed -n '84,88p' services/cms/package.jsonRepository: rage/secret-project-331
Length of output: 1923
Align @types/node major version with the pinned Node runtime.
Line 86 pins @types/node to ^25.3.3, but the workspace configuration (pnpm-workspace.yaml) specifies nodeVersion: 24.14.0. The types major version should match the runtime major version to prevent type incompatibilities and mask runtime issues. Change to ^24.14.0.
Suggested patch
- "@types/node": "^25.3.3",
+ "@types/node": "^24.14.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "@types/node": "^25.3.3", | |
| "@types/node": "^24.14.0", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/cms/package.json` at line 86, The package.json dependency
"@types/node" is pinned to ^25.3.3 which mismatches the workspace Node runtime
(nodeVersion 24.14.0); update the "@types/node" entry in services/cms
package.json to use the matching major/minor version (e.g., ^24.14.0) so the
types align with the pinned runtime and avoid type/runtime incompatibilities.
| "@types/react": "^19.2.0", | ||
| "@types/react-dom": "^19.2.0", | ||
| "@types/lodash": "^4.17.24", | ||
| "@types/node": "^25.3.3", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Node version pins =="
fd 'pnpm-workspace.yaml$' | while read -r f; do
echo "-- $f"
rg -n 'nodeVersion:' "$f" || true
done
echo
echo "== `@types/node` in services/main-frontend/package.json =="
rg -n '"@types/node"\s*:' services/main-frontend/package.jsonRepository: rage/secret-project-331
Length of output: 807
@types/node major should match the pinned Node runtime major.
At line 88, @types/node is ^25.3.3 (Node 25.x) while the workspace config is pinned to Node 24.14.0; this can allow invalid runtime API usage during type-checking. Update @types/node to ^24.x or align the runtime version.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/main-frontend/package.json` at line 88, The dev dependency
"@types/node" is set to ^25.3.3 but the workspace Node runtime is pinned to
24.14.0; update the entry for "@types/node" in package.json to use a 24.x caret
range (e.g., "^24.0.0" or "^24.x") so the type definitions match the pinned Node
major and prevent invalid API type-checking against Node 25.
| "katex": "^0.16.33", | ||
| "lodash": "^4.17.23", | ||
| "monaco-editor": "^0.55.1", | ||
| "next": "16.1.6", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify current package settings
echo "=== Checking Next.js version in package.json ==="
rg -n '"next"\s*:\s*"16\.1\.6"' services/quizzes/package.json || echo "Version not found"
echo -e "\n=== Checking for 'next export' in scripts ==="
rg -n '"export"\s*:\s*"next export"' services/quizzes/package.json || echo "Export script not found"
echo -e "\n=== Checking for next.config files ==="
fd -HI '^next\.config\.(js|mjs|cjs|ts)$' services/quizzes 2>/dev/null || echo "No next.config found in services/quizzes"
# Check if output: "export" is configured anywhere in the services directory
echo -e "\n=== Searching for output export configuration ==="
if [ -d "services/quizzes" ]; then
find services/quizzes -maxdepth 2 -name "next.config.*" -exec cat {} + 2>/dev/null | head -50 || echo "No config files found"
fiRepository: rage/secret-project-331
Length of output: 1474
🌐 Web query:
Next.js 16.1.x static export CLI command documentation - is "next export" still supported or deprecated
💡 Result:
In Next.js 16.1.x, the next export CLI command is not supported (it was deprecated in v13.3.0 and removed in v14.0.0). Instead, static exports are done by setting output: 'export' in next.config.js and then running next build, which produces the static output (default out/). [1]
If you still have scripts like next build && next export, you should remove next export and rely on output: 'export' + next build. [1]
Sources
[1] Next.js Docs – “Static Exports” (Last updated Feb 27, 2026), incl. version history for next export deprecation/removal: (nextjs.org)
Citations:
The next export command is not supported in Next.js 16.1.6 and will fail—this needs to be fixed.
Next.js removed the next export CLI command in v14.0.0. The "export": "next export" script on line 8 is incompatible with v16.1.6. Additionally, next.config.js is configured with output: "standalone" (for server deployments), not output: "export" (for static exports).
Either remove the export script if static exports aren't needed, or change output: "standalone" to output: "export" in next.config.js and use next build to generate static output.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/quizzes/package.json` at line 47, The package.json contains an
unsupported "export": "next export" npm script (and next.config.js currently
uses output: "standalone"); either remove the "export" script and any tooling
invoking it if you don't need static exports, or change next.config.js output to
"export" and replace the script with a build-based flow (use "next build" and
then serve the generated static output) so you stop calling the removed CLI;
update the package.json scripts accordingly and ensure next.config.js's output
setting (the output property) and any deployment steps align (refer to the
"export" script in package.json and the output property in next.config.js).
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
services/headless-lms/models/src/user_chapter_locking_statuses.rs (2)
209-307: Add rustdoc for the new public chapter-locking APIs.
complete_and_lock_chapterandget_or_init_all_for_courseintroduce important side effects (state mutation + initialization/unlock behavior) but are undocumented. Add short docs clarifying intent and side effects.As per coding guidelines, “Require consistent, minimal function documentation… Public interfaces should have clearer and more comprehensive documentation.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/headless-lms/models/src/user_chapter_locking_statuses.rs` around lines 209 - 307, The two public functions complete_and_lock_chapter and get_or_init_all_for_course lack rustdoc describing intent and side effects; add concise /// comments above each function that state what the function does, its parameters, the side effects (DB mutations, potential creation of rows and unlocking behavior), the returned value (UserChapterLockingStatus or Vec<UserChapterLockingStatus>), and error/ not-found behaviors (ModelError on failure); ensure docs mention when course chapter locking must be enabled and that get_or_init_all_for_course will insert missing rows and may call unlock_first_chapters_for_user, and keep each docblock short (one to three sentences) following project doc style.
245-260: Align ON CONFLICT target to use constraint name for consistency.The insert paths at lines 245–260 and nearby use
ON CONFLICT (user_id, chapter_id, deleted_at)while other mutations in the same file useON CONFLICT ON CONSTRAINT idx_user_chapter_locking_statuses_user_chapter_active. The constraint definition isUNIQUE NULLS NOT DISTINCT (user_id, chapter_id, deleted_at), so both forms are functionally equivalent—PostgreSQL correctly infers the same constraint either way. However, using the explicit constraint name form consistently throughout improves code clarity and reduces reliance on inference.Suggested changes
-ON CONFLICT (user_id, chapter_id, deleted_at) DO NOTHING +ON CONFLICT ON CONSTRAINT idx_user_chapter_locking_statuses_user_chapter_active DO NOTHINGApply the same change to all insert operations in this file (lines 245 and nearby).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/headless-lms/models/src/user_chapter_locking_statuses.rs` around lines 245 - 260, Replace the positional ON CONFLICT target with the explicit constraint name to match other mutations: in the INSERT query that currently uses "ON CONFLICT (user_id, chapter_id, deleted_at) DO NOTHING" (the query that inserts into user_chapter_locking_statuses and selects from chapters) change it to "ON CONFLICT ON CONSTRAINT idx_user_chapter_locking_statuses_user_chapter_active DO NOTHING"; apply the same replacement to the other insert statements in this file so all inserts consistently reference idx_user_chapter_locking_statuses_user_chapter_active instead of the column list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@services/headless-lms/models/src/user_chapter_locking_statuses.rs`:
- Around line 209-307: The two public functions complete_and_lock_chapter and
get_or_init_all_for_course lack rustdoc describing intent and side effects; add
concise /// comments above each function that state what the function does, its
parameters, the side effects (DB mutations, potential creation of rows and
unlocking behavior), the returned value (UserChapterLockingStatus or
Vec<UserChapterLockingStatus>), and error/ not-found behaviors (ModelError on
failure); ensure docs mention when course chapter locking must be enabled and
that get_or_init_all_for_course will insert missing rows and may call
unlock_first_chapters_for_user, and keep each docblock short (one to three
sentences) following project doc style.
- Around line 245-260: Replace the positional ON CONFLICT target with the
explicit constraint name to match other mutations: in the INSERT query that
currently uses "ON CONFLICT (user_id, chapter_id, deleted_at) DO NOTHING" (the
query that inserts into user_chapter_locking_statuses and selects from chapters)
change it to "ON CONFLICT ON CONSTRAINT
idx_user_chapter_locking_statuses_user_chapter_active DO NOTHING"; apply the
same replacement to the other insert statements in this file so all inserts
consistently reference idx_user_chapter_locking_statuses_user_chapter_active
instead of the column list.
nygrenh
force-pushed
the
update-deps-2026-03
branch
from
ade8aed to
c7e85b6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores